-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ui/cluster-ui: fix no most recent stmt for active txns #88179
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are there any tests you can add to confirm the fix?
Reviewable status: complete! 0 of 0 LGTMs obtained
I can add tests to confirm the field will be populated based on the sessions response. But actually, with this change we will now show the latest query text for an active txn even if that txn is not currently executing anything. Is that really what we want here? Or should we somehow signal the txn isn't currently executing a query while showing the latest stmt text? Currently the only signal is from the fact the statement id link isn't available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's okay, the label it's already "most recent", so it doesn't mean it has (or not) to be something active
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @xinhaoz)
I was thinking more about in the active transaction details page, where it might be more confusing |
9cf1967
to
8da8f7f
Compare
Fixes cockroachdb#87738 Previously, active txns could have an empty 'Most Recent Statement' column, even if their executed statement count was non-zero. This was due to the most recent query text being populated by the active stmt, which could be empty at the time of querying. This commit populates the last statement text for a txn even when it is not currently executing a query. This commit also removes the `isFullScan` field from active txn pages, as we cannot fill this field out without all stmts in the txn. Release note (ui change): Full scan field is removed from active txn details page. Release note (bug fix): active txns with non-zero executed statement count now always have populated stmt text, even when no stmt is being executed.
8da8f7f
to
6deea40
Compare
bors r+ |
Build succeeded: |
Fixes #87738
Previously, active txns could have an empty 'Most Recent Statement' column, even if their executed statement count was non-zero. This was due to the most recent query text being populated by the active stmt, which could be empty at the time of querying. This commit populates the last statement text for a txn even when it is not currently executing a query.
This commit also removes the
isFullScan
field from active txn pages, as we cannot fill this field out without all stmts in the txn.Release note (ui change): Full scan field is removed from active txn details page.
Release note (bug fix): active txns with non-zero
executed statement count now always have populated stmt text, even when no stmt is being executed.